[papi] Add UserService/UpdateUser to proto
#19163
Conversation
AlexTugarev
force-pushed
the
at/papi-adduser-proto
branch
from
985859f to
4846ad6
Compare
| } | ||
| repeated WorkspaceAutostartOption workspace_autostart_options = 8; | ||
| message WorkspaceAutostartOption { | ||
| string clone_url = 1; |
There was a problem hiding this comment.
all properties should be optional then
There was a problem hiding this comment.
@akosyakov the workspace_autostart_options is a repeated field. The way you would update it is to provide a complete new array of values of type WorkspaceAutostartOption. Signaling optional here would suggest the system could update an individual entry by specifying a diff, but how should that be implemented?
The simpler solution is, if workspace_autostart_options is not provided (it's optional anyways), we're not touching this in persistent state. If it's an empty array, we reset the state. If there are entries, we store this as new state.
There was a problem hiding this comment.
We discussed to move it out to own SetWorkspaceAutostartOptions which is not partial and support current dashboard.
There was a problem hiding this comment.
resolved via 1a0632b
|
|
||
| // SetWorkspaceAutoStartOptions updates the auto start options for the Gitpod Dashboard. | ||
| // +internal - only used by the Gitpod Dashboard. | ||
| rpc SetWorkspaceAutoStartOptions(SetWorkspaceAutoStartOptionsRequest) returns (SetWorkspaceAutoStartOptionsResponse) {} |
There was a problem hiding this comment.
Do we really need this method? I thought autoStart option is not useful, and can cause some confuse for me (Once saw that we want to remove it, I'm happy with that Closed PR XD, but closed already).
There was a problem hiding this comment.
I'm not aware of removing auto start. Do we have an issue or any discussion on that?
There was a problem hiding this comment.
Sorry @AlexTugarev , this is not-blocking comment.
This is the PR to remove auth start option. I'm not clear if we want to keep it or remove it.
update: I just prefer to remove the auto start options
There was a problem hiding this comment.
It's still used to preserve the options for the next workspace start.
There was a problem hiding this comment.
Also, there is the autostart options to the prefixed URL.
| optional WorkspaceTimeoutSettings workspace_timeout_settings = 8; | ||
| message WorkspaceTimeoutSettings { | ||
| optional google.protobuf.Duration inactivity = 1; | ||
| optional bool disabled_disconnected = 2; |
There was a problem hiding this comment.
we have another workspace timeout in Refind workspace proto PR https://github.com/gitpod-io/gitpod/pull/19138/files#diff-393d8178f08b8814eea780cf779ac93bc55fd0b90588e907a825cbb51de9f33aR250
Should them be aligned?
There was a problem hiding this comment.
Let's align them in implementation PR.
TBH, this is already confusing enough. Changed that already twice, and it's unclear how to map them. In the beginning we'll need to map them in order to maintain compatibility in FE shim, right?
Here I did the bare minimum for the conversion.
https://github.com/gitpod-io/gitpod/pull/19142/files#diff-8fdb520274403fe903bd0ad9c9094f3951b4940b9cf838aa5c36b6548f9aafbdR964-R967
Happy to align, if there is an example for the conversion available.
|
/unhold |
Description
This PR just adds
UserService/UpdateUser.The attributes needs to be modifiable were identified in #19142.
How to test
Documentation
Preview status
gitpod:summary
Build Options
Build
Run the build with werft instead of GHA
Run Leeway with
--dont-testPublish
Installer
Add desired feature flags to the end of the line above, space separated
Preview Environment / Integration Tests
If enabled this will build
install/previewIf enabled this will create the environment on GCE infra
Saves cost. Untick this only if you're really sure you need a non-preemtible machine.
Valid options are
all,workspace,webapp,ide,jetbrains,vscode,ssh. If enabled,with-previewandwith-large-vmwill be enabled./hold